Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix ToUint64E and ToUint64 overflow, e.g. "18446744073709551615"(math.MaxUint64) #175

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix ToUint64E and ToUint64 overflow, e.g. "18446744073709551615"(math.MaxUint64) #175

wants to merge 1 commit into from

Conversation

morrisxyang
Copy link

cast.ToUint64E("18446744073709551615") // math.MaxUint64
will get err about value out of range. For ToUint64E, using strconv.ParseUint instead of strconv.ParseInt.

@morrisxyang
Copy link
Author

@sagikazarmark

@morrisxyang morrisxyang changed the title fix ToUint64E overflow, e.g. "18446744073709551615"(math.MaxUint64) fix ToUint64E and ToUint64 overflow, e.g. "18446744073709551615"(math.MaxUint64) Mar 7, 2023
@morrisxyang
Copy link
Author

For code consistency, other uint cast methods are also changed to use strconv.ParseUint. tests look good.

@bep
Copy link
Collaborator

bep commented Mar 7, 2023

@Imatvoid you need to cool off with these "mentions" (and emails).

@morrisxyang
Copy link
Author

ok

@morrisxyang
Copy link
Author

morrisxyang commented Mar 8, 2023 •
edited
Loading

I saw this #140, but seemingly changing strconv.ParseUInt to strconv.ParseInt in the ToUint64E method led to an overflow problem. Or is there something I understand wrong? I’m eager to receive feedback.

@sagikazarmark
Copy link
Collaborator

The problem is on our radar, but it's a little bit more complicated than that.

We will provide a solution. Until then, using a fork should be relatively easy in Go.

@morrisxyang
Copy link
Author

The problem is on our radar, but it's a little bit more complicated than that.

We will provide a solution. Until then, using a fork should be relatively easy in Go.

Thank you for your reply, actually, I'm interested in the "a little bit more complicated" problem. I will pay attention to the progress and try to learn something. It would be better if I could help.

@rickywei
Copy link

rickywei commented Aug 4, 2023

The problem is on our radar, but it's a little bit more complicated than that.

We will provide a solution. Until then, using a fork should be relatively easy in Go.

Hi @sagikazarmark, any updates with this one?

@blacktop blacktop mentioned this pull request Jun 6, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@morrisxyang @bep @sagikazarmark @rickywei